feat(dincli): rewards, encryption-key, release-slots and dispute commands (task_061026_21 Part C) - #302

Reviewed head 15e7afc3f274926fa5f87671ddc351e5a51cc51c against develop / merge-base e9424af1db5c17fabb0919253954ba5f435337aa: two commits ahead, zero behind. At the October 29, 18:09 UTC refresh, GitHub reported open/mergeable and no discussion comments. Recheck before posting.

Changes requested

1. P1 — Block funding a settled GI. Target selection accepts the current GI after settlement. depositRewards accepts it, but claims use the frozen settlement snapshot: additional DIN never becomes claimable, and emission funding also consumes a schedule step. Fix: reject giRewardSnapshot(target).settled before approval/funding; test both deposit and emission against a settled current GI. Evidence: CLI/contract inspection, not chain reproduction.

2. P1 — Preserve the last confirmed test-data secret. Persistence overwrites the sole GI/batch file before assignment. Following partial success or flag failure, a retry can save K2 and then fail its replacement transaction, leaving K1 committed on-chain but lost locally. resolve-test-data then reveals the mismatch, upholding the dispute and potentially penalizing rewards. Fix: retain commitment-indexed pending/confirmed secrets and verify the on-chain commitment before revealing. Add success → retry failure → resolve regressions. An offline probe of the exact extracted helpers confirmed overwrite behavior with a simulated failed transaction; no full CLI/chain flow was run.

3. P2 — Create secrets with 0600 immediately. Auditor-key creation and batch-secret writes precede chmod. With umask 022, files can initially be 0644; interruption leaves exposure. Fix: safe exclusive/atomic creation with 0600 at open. Test creation-time permissions and interruption, not just final mode. Evidence: source inspection.

Claimed / verified

• Rewards/emission, claims, slots and model-ID checks: relevant selectors, getter positions and guards align statically; mock tests cover arguments/order/early exits, not balances or contract execution. Finding 1 remains.
• Keys, saved K/hash and shared encryption: implemented; wallet separation/reuse and final permissions have mock coverage. Findings 2–3 remain; real decryption, retry recovery and encrypted-output equivalence were not verified.
• Disputes and fail-fast checks: source/mocks support bond-before-open, seed-before-uphold, selected state/windows and keyless-auditor refusal. Reassignment, recomputation, slashing and timeout recovery lack independent chain evidence.
• Helper reuse/ABI: reuse and relevant signatures checked; artifact generation/full ABI equality not reproduced.
• Tests: CI run has Python, Solidity, Docs and CI OK green. Its nonblocking full-GI job failed on the existing platform proxyAdmin assertion before new flows. Author-reported 609 passes were not independently reproduced. No local full pytest/build/live-chain/IPFS run; dependencies unavailable.

Scope/process: deploy follow-up, documentation/artifact paths and Part D remain outstanding; keep issue No. 223 open. Neither commit has closing keywords; prefer “Part of issue No. 223; addresses BL-28” in the body. Authenticated peer helper, isolated Git worktree and local merge-tree check were not run: this is not a completed formal skill run or eligibility certification. umeradl and author umermjd11 are one person; obtain an eligible independent peer. No contract-source changes trigger the automatic second-signoff rule here; any later contract/mechanism fix needs its own review.

Recommendation: request changes on the three findings, then recheck regressions, final SHA/CI and live-flow coverage.

23 files; +2105/-63, at the head/base above. Global table notes: base equals merge-base (no intervening base drift); GitHub reports mergeable: true, but supplied no mergeStateStatus. Local conflicts were not checked, so local/GitHub agreement is unestablished. “Base unchanged”, “Not checked” and “None reported” below refer to these global facts—not per-file local verification. All actual proposals remain Soon.

Developer/BACK_LOG.md

dincli/abis/DinEmission.json

dincli/cli/aggregator.py

dincli/cli/auditor.py

dincli/cli/context.py

dincli/cli/dinrep.py

dincli/cli/dintoken.py

dincli/cli/modelowner.py

dincli/cli/modelownerd/auditor_batches.py

dincli/cli/modelownerd/disputes.py

dincli/cli/modelownerd/gi.py

dincli/cli/modelownerd/rewards.py

dincli/cli/rewards.py

dincli/cli/utils.py

dincli/main.py

tests/_cli_fakes.py

tests/test_auditor_encryption_key.py

tests/test_create_testdataset_submit.py

tests/test_dinrep_approve_model_id.py

tests/test_dintoken.py

tests/test_dispute_commands.py

tests/test_gi_release_slots_and_funding.py

tests/test_rewards_commands.py

Verification

Comment 1 distinguishes observed CI, source/mocks and the extracted-helper simulation. No full local suite or live flow was run; full-GI CI failed before these commands. Local/GitHub conflict agreement: not established. Resolve findings 1–3 and recheck eligibility/head/CI before approval.

Thanks Umer! These commands are a useful step forward. I’d address the three funding and secret-handling findings, add the regression tests, and then recheck before merging. The remaining live-flow verification is noted above.

Reviewed against develop in an isolated worktree (pr-302-review on top of 15e7afc3, the PR head). Peer check: OK (umermjd11 is the author; operator is not).

CI on 15e7afc3: CI OK ✅, Python ✅ (609 passed, 133 deselected), Solidity ✅, Docs ✅ — all green. Integration (full GI) failed, but that is explicitly non-blocking per MAINTAINING No. 4.9 and the PR body ("Not run against a chain. The full-GI integration run is Part D (PR 3)").

The PR's own work is two commits: 01c26e0b (all new commands, +2103/-61 lines) and 15e7afc3 (BACK_LOG.md only). The remaining diff against upstream/develop is from upstream merge commits already on develop.

Claimed — all 15 new command paths are registered. Verified via test_command_is_registered, which invokes CliRunner().invoke(main_app, [..., "--help"]) on every path and asserts exit_code == 0. Wiring: dincli/cli/rewards.py (claim/withdraw, any role) added at the top level via main.py; dincli/cli/modelownerd/rewards.py (deposit/fund-emission, model owner) and dincli/cli/modelownerd/disputes.py added under model-owner via modelowner.py. Both the top-level rewards sub-app and the model-owner rewards sub-app exist and are routed to different Typer apps.

Claimed — approve_din helper is extracted and reused by stake, deposit and dispute-bond commands. Verified: approve_din added in dintoken.py; it calls din_token.functions.approve(spender, amount) followed by time.sleep(5). stake_dintokens now uses it (replacing the same inline block). All three callers that need a DIN pull — reward deposit, test-data dispute bond, aggregation dispute bond — go through it.

Claimed — gi start fails fast on an empty reward pool before the pass-score evaluation. Verified in gi.py L~93–100: immediately after the GI-index check and before the manifest policy section, the code reads giRewardPool(target_gi) and exits with a user-friendly message pointing to rewards deposit / fund-emission if zero. Test: test_gi_start_exits_early_if_reward_pool_is_empty in test_gi_release_slots_and_funding.py asserts no tx is sent and the message contains "reward pool is empty."

Claimed — resolve-aggregation --upheld locks the dispute seed before calling resolveDispute. Verified in disputes.py _lock_dispute_seed: polls until disputes(...)[8] != ZERO_BYTES32, waits for seedBlock, then calls lockDisputeSeed with exit_on_failure=False; if the receipt is None and the seed is still zero, raises Exit. build_and_send_tx with exit_on_failure=False returns None on any failure, confirmed at L1008–1035 of utils.py. Test: test_resolve_aggregation_upheld_locks_the_seed_first uses a callable views["disputes"] that returns a locked seed only after lockDisputeSeed appears in sent; asserts [lockDisputeSeed, resolveDispute] in order.

Claimed — one X25519 key per wallet. Verified: _auditor_x25519_key_path(address) returns auditor_x25519_<checksum_address>.key; _load_auditor_x25519_key(address) uses it; the old single-file path is gone. key_path.chmod(0o600) is called on every code path through register-encryption-key. The owner key owner_x25519.key is similarly chmod'd 0o600 via _load_owner_x25519_key.

Claimed — per-batch K and plaintext hash saved before the assign tx. Verified in auditor_batches.py: _save_testdata_secret writes {"K": hex, "plaintext_keccak": hex} to audits/testdata_keys/gi_<gi>_batch_<b>.json and calls .chmod(0o600) before returning. create-testdataset --submit calls it inside the batch loop, before build_and_send_tx(assignAuditTestDataset). _load_testdata_secret is used by resolve-test-data to recover K for the on-chain reveal.

Claimed — create-testdataset --submit exits before any tx if any batch auditor is missing an encryption key. Verified: _auditors_missing_encryption_key collects all addresses with len(getEncryptionKey(addr).call()) == 0 across all batches to process, raises Exit with their addresses if non-empty, and does so before the first assignAuditTestDataset call. Test: test_create_testdataset_exits_before_any_tx_if_an_auditor_key_is_missing.

Claimed — dinrep registry approve-registration-request reads modelId() from both task contracts and compares to totalModels(). Verified: _task_contract_model_ids reads req[3] (taskCoordinator) and req[4] (taskAuditor) from the modelRequests(requestId) tuple. Struct layout in DINModelRegistry.sol L85–95: [0]requester [1]isOpenSource [2]manifestCID [3]taskCoordinator [4]taskAuditor .... The indexing is correct. Mismatching contracts are named individually in the error message. --force bypasses the check. Test: test_dinrep_approve_model_id.py covers mismatch (refusal), force, and matching cases.

Claimed — DinEmission.json ABI is sourced from foundry/out. Verified: fundGI, giEmissionFunded, and emissionForCurrentEpoch all appear in the ABI and all exist in foundry/src/DinEmission.sol (L212, L250, and a view getter respectively). get_deployed_din_emission_contract reads the "emission" key from din_info (consistent with how all other platform contract getters resolve addresses).

Claimed — BACK_LOG.md BL-28 updated. Verified: BL-28 changed from "🆕 New — unaddressed" to "🟡 Fix in review — PR No. 302 adds dincli model-owner gi release-slots." BL-11 updated to note that model-owner disputes resolve-aggregation --upheld now locks the seed itself.

No findings. Merge-ready; branch needs update-branch before merge (BEHIND develop, no conflict). No foundry/src/** touched, so no second sign-off is required.

Head at review: 15e7afc3
Merge base (upstream develop): behind but clean — no conflict (merge-tree exit 0)

dincli/abis/DinEmission.json (new)

dincli/cli/context.py

dincli/cli/dintoken.py

dincli/cli/utils.py

dincli/cli/modelownerd/rewards.py (new)

dincli/cli/rewards.py (new)

dincli/cli/modelownerd/gi.py

dincli/cli/auditor.py

dincli/cli/modelownerd/auditor_batches.py

dincli/cli/modelownerd/disputes.py (new)

dincli/cli/aggregator.py

dincli/cli/dinrep.py

dincli/cli/modelowner.py / dincli/main.py

tests/_cli_fakes.py (new) / tests/test_rewards_commands.py / tests/test_dispute_commands.py / tests/test_gi_release_slots_and_funding.py / tests/test_auditor_encryption_key.py / tests/test_dinrep_approve_model_id.py / tests/test_create_testdataset_submit.py

tests/test_dintoken.py

Developer/BACK_LOG.md

@robertocarlous, I merged develop into this branch to clear the conflict, so the new head is 5e3b99cd. The push dismissed your approval of 15e7afc3.

• The only conflict: two rows in Developer/BACK_LOG.md.
  • BL-27 now takes develop's "✅ Fixed — PR No. 261", since 261 merged on Nov 2.
  • BL-28 keeps this PR's "Fix in review" status.
• No other changes: every other file has exactly the changes you approved.
• Tests: pytest -m "not integration" passes on the merged tree (741 passed).

Once CI OK is green on 5e3b99cd, could you re-approve and click Merge when ready? I can't queue it myself, because the peer check refuses me on my own PR.